I tried adding singleTask to this activity for a deeplink issue and it caused a bunch of odd behaviours (skipping deeplinks, activity not fully loading), have you noticed anything strange after the change?

I didn't see any problem on HomeActivity after this change but I didn't stress it. I would prefer to make more tests.

Indeed, clicking on a link like https://app.element.io/#/room/%23element-web-announcements:matrix.org

creates 2 tasks if Element was already running, so we can see in the task switcher:

See #2299 and the fix #2341 :)

is this a blocker for the fix? maybe singleTop could provide similar protection without allowing multiple instances

No more needed to change launchMode since we generate a random task affinity on each build.

would be interesting to know if there's a performance impact for this, might be worth making the check async to avoid slowing down the activity start flow

Done

looks like we could extract this logic to reduce the duplication, either extension or base class function

something like

if (views.drawerLayout.isDrawerOpen(GravityCompat.START)) { 
  views.drawerLayout.closeDrawer(GravityCompat.START)
} else {
  validateBackPressed { super.onBackPressed() }
}

...

fun Activity.validateBackPressed(onBackPressed: () -> Unit) {
    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R && supportFragmentManager.backStackEntryCount == 0) {
        if (isTaskRoot) {
            onBackPressed()
        } else {
            Timber.e("Application is potentially corrupted by an unknown activity")
            finishAffinity()
        }
    } else {
        onBackPressed()
    }
}

Done

Please add documentation to explain that this should be only used for top-level activities. A developer which is not aware of this could think that it's a good practice to use it for all activities.

Done

Funny: I had the same mental path "A developer which is not aware of this could think that it's a good practice to use it for all activities". And now I see this comment in GitHub, and the comment in the code :)

Maybe using 8 chars in [a-z] is enough :)

done

Maybe rename to generateTaskAffinitySuffix

done

not a blocker since we're not using the build cache, wanted to highlight generating new values on each build/gradle configuration cycle may cause some of the android gradle tasks to become non cachable

Ah, something else I did not think about.
We want to have stable builds, i.e. the same code generates the same binary (related: #842, and context in element-hq/riot-android#3131). It's important for F-Droid.
With this change it will not be the case anymore.
I have not idea how to handle that now.

could we use the short git hash for the suffix? 🤔

We should propably compute the task affinity suffix from tags or commit hash ?

could we use the short git hash for the suffix? 🤔

We can't use it as is. taskAffinity has same format as namespace :

• It must have at least two segments (one or more dots).
• Each segment must start with a letter.
• All characters must be alphanumeric or an underscore [a-zA-Z0-9_].

https://developer.android.com/studio/build/configure-app-module#set-application-id

we could use the git hash as a seed for the random()

That's a good idea to compute a suffix using git hash and just remove forbidden chars (but is there any?).

So many having a appTaskAffinitySuffix like a${git_hash_8_digits} with a a prefix to ensure it's always starting with a letter would fit all our needs. WDYT @yostyle ?

we already have gitRevision. I'm going to reuse it.